Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Referral letter preview desktop #8954

Open
wants to merge 17 commits into
base: develop
Choose a base branch
from

Conversation

modamaan
Copy link
Contributor

@modamaan modamaan commented Oct 28, 2024

Proposed Changes

Referral letter preview desktop and responsive

Before
Image

After
Screenshot 2024-10-29 064626

@ohcnetwork/care-fe-code-reviewers

Merge Checklist

  • Add specs that demonstrate bug / test a new feature.
  • Update product documentation.
  • Ensure that UI text is kept in I18n files.
  • Prep screenshot or demo video for changelog entry, and attach it to issue.
  • Request for Peer Reviews
  • Completion of QA

Summary by CodeRabbit

  • New Features

    • Enhanced print functionality for referral letters with improved layout and readability.
    • Introduced a new UI card system for better presentation of content.
    • Added a QR code to the printed layout encoding the URL for the shifting record.
  • Bug Fixes

    • Minor adjustments to the print mode toggle and button layout for improved usability.

@modamaan modamaan requested a review from a team as a code owner October 28, 2024 15:19
Copy link

netlify bot commented Oct 28, 2024

Deploy Preview for care-ohc ready!

Name Link
🔨 Latest commit a4f329a
🔍 Latest deploy log https://app.netlify.com/sites/care-ohc/deploys/672bb2443eeb110008cf7a60
😎 Deploy Preview https://deploy-preview-8954--care-ohc.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@github-actions github-actions bot added the Deploy-Failed Deplyment is not showing preview label Oct 28, 2024
@modamaan
Copy link
Contributor Author

@bodhish should i update anything?

@modamaan
Copy link
Contributor Author

Screenshot 2024-10-29 192122

@modamaan
Copy link
Contributor Author

@rithviknishad can you check

src/CAREUI/misc/PrintPreview.tsx Outdated Show resolved Hide resolved
@rithviknishad rithviknishad added changes required and removed Deploy-Failed Deplyment is not showing preview labels Oct 30, 2024
@nihal467
Copy link
Member

nihal467 commented Nov 4, 2024

image
@modamaan

  • the close and print buttons in the mobile responsive view is not properly aligned

@bodhish
Copy link
Member

bodhish commented Nov 6, 2024

@coderabbitai full review

Copy link

coderabbitai bot commented Nov 6, 2024

✅ Actions performed

Full review triggered.

Copy link

coderabbitai bot commented Nov 6, 2024

Walkthrough

The changes in this pull request enhance the ShiftDetails.tsx component by improving the print functionality and layout of the referral letter. A new PrintPreview component is introduced, along with a card layout for better organization of printed content. The QR code implementation is updated to QRCodeSVG, and various UI components from a card library are added. Additionally, a new set of card components is created in card.tsx, providing a structured way to display card content.

Changes

File Change Summary
src/components/Shifting/ShiftDetails.tsx Enhanced print functionality with PrintPreview, replaced QRCode with QRCodeSVG, improved layout and styling.
src/components/ui/card.tsx Introduced new card components: Card, CardHeader, CardTitle, CardDescription, CardContent, CardFooter.

Assessment against linked issues

Objective Addressed Explanation
Fix referral letter responsiveness (#8884)

🐇 In the garden, I hop and play,
With cards and prints, we brighten the day!
A QR code shines, a layout so neat,
For every referral, a joy to greet!
Hooray for the changes, let’s dance and cheer,
A clean, responsive letter, oh dear! 🌼


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (5)
src/components/ui/card.tsx (3)

5-18: Add JSDoc documentation for better developer experience.

While the component implementation is solid, adding JSDoc documentation would improve its usability.

Add documentation like this:

+/**
+ * A card component that provides a consistent container with styling.
+ * @param className - Additional classes to be applied to the card
+ * @param props - HTML div element props
+ */
const Card = React.forwardRef<
  HTMLDivElement,
  React.HTMLAttributes<HTMLDivElement>
>(({ className, ...props }, ref) => (

20-74: Consider centralizing padding management.

The current implementation duplicates padding logic across CardHeader, CardContent, and CardFooter components. Consider using CSS composition or CSS custom properties to manage these spacings centrally.

Example approach:

  1. Define CSS custom properties for card spacings
  2. Use these in your components for consistent spacing management
  3. This would make it easier to maintain and modify spacings globally
/* In your CSS */
:root {
  --card-padding: 1.5rem;
  --card-spacing: 1.5rem;
}

Then use these variables in your className definitions.


76-83: Reorder exports to match declaration order.

For better maintainability, consider matching the export order with the declaration order of components.

export {
  Card,
  CardHeader,
  CardTitle,
  CardDescription,
  CardContent,
  CardFooter,
-  CardTitle,
-  CardDescription,
-  CardContent,
};
src/components/Shifting/ShiftDetails.tsx (2)

432-433: Improve responsiveness of inline styles for print mode close button.

The inline styles using fixed pixel values (top-[190px], mr-[100px]) may not scale well on different screen sizes. Consider using responsive design practices with Tailwind CSS classes to ensure consistent positioning across devices.

Refactor the styles using responsive utility classes:

-<div className="relative top-[190px] my-5 mr-[100px] flex justify-end gap-3 xl:absolute xl:right-6 xl:top-7 xl:justify-end">
+<div className="relative my-5 flex justify-end gap-3 xl:absolute xl:right-6 xl:top-7 xl:justify-end">

Adjust margins and positioning with responsive classes like mt-5, mr-5, top-5, etc., to maintain layout integrity.


449-449: Enhance accessibility by adding ARIA labels to buttons.

To improve accessibility, add aria-label attributes to buttons in the options prop so screen readers can convey the purpose of the buttons to users.

Update the ButtonV2 components:

<ButtonV2
+  aria-label={t("update_status_details")}
  ...
<ButtonV2
+  aria-label={t("referral_letter")}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between bc15ed9 and 39a5f15.

📒 Files selected for processing (2)
  • src/components/Shifting/ShiftDetails.tsx (4 hunks)
  • src/components/ui/card.tsx (1 hunks)
🔇 Additional comments (4)
src/components/ui/card.tsx (2)

1-4: LGTM! Clean and minimal imports.

The imports are well-organized and include only the necessary dependencies.


1-83: Verify integration with referral letter preview.

The card components look well-designed for improving the referral letter layout. Let's verify the integration:

✅ Verification successful

Card components are properly integrated in ShiftDetails and across the application

The verification confirms that:

  • Card components are correctly imported in ShiftDetails.tsx with the necessary subcomponents (Card, CardContent, CardHeader, CardTitle)
  • The components are properly used in ShiftDetails.tsx with appropriate layout classes and hierarchy
  • The card components have been widely adopted across the application, indicating consistent UI patterns
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of card components in ShiftDetails.tsx
# Expected: Find card components being used in the referral letter preview

# Search for card component imports and usage
rg -A 5 "import.*from.*card" 
rg "<Card" 

Length of output: 2571

src/components/Shifting/ShiftDetails.tsx (2)

305-421: Ensure dynamic data is correctly rendered in the new print layout.

The refactored printData function now uses PrintPreview and Card components for a cleaner layout. Verify that all dynamic fields, especially patient and consultation details, are accurately displayed in the printed referral letter.

Run the following script to check for placeholders or missing data bindings:

#!/bin/bash
# Description: Search for any placeholders or missing dynamic data in the printData function.

rg '\{.*\}' src/components/Shifting/ShiftDetails.tsx -A 5 -B 5

8-9: Verify import paths for new components.

You have added imports for PrintPreview and card components. Ensure that the paths @/CAREUI/misc/PrintPreview and @/components/ui/card are correct and that these components exist at these locations to prevent import errors.

Run the following script to confirm the existence of the imported files:

✅ Verification successful

Import paths are correct and components exist

Both imported components exist at the specified locations:

  • PrintPreview component exists at src/CAREUI/misc/PrintPreview.tsx
  • Card components exist at src/components/ui/card.tsx

The imports are correctly referenced using the @/ alias which is a common pattern for absolute imports from the source directory.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if the imported components exist.

# Verify PrintPreview component
if [ ! -f "./src/CAREUI/misc/PrintPreview.tsx" ]; then
  echo "PrintPreview component not found at '@/CAREUI/misc/PrintPreview'."
else
  echo "PrintPreview component found."
fi

# Verify Card components
if [ ! -f "./src/components/ui/card.tsx" ]; then
  echo "Card components not found at '@/components/ui/card'."
else
  echo "Card components found."
fi

Length of output: 27006


Script:

#!/bin/bash
# Let's verify the existence of the imported components using fd and rg

# Check for PrintPreview component
fd "PrintPreview.tsx" src/CAREUI/misc/

# Check for card components
fd "card.tsx" src/components/ui/

# Additionally, let's verify the actual imports in these files if they exist
rg "export.*PrintPreview" -A 2
rg "export.*Card" -A 2

Length of output: 5905

src/components/ui/card.tsx Outdated Show resolved Hide resolved
src/components/Shifting/ShiftDetails.tsx Show resolved Hide resolved
@github-actions github-actions bot added the merge conflict pull requests with merge conflict label Nov 6, 2024
Copy link

github-actions bot commented Nov 6, 2024

👋 Hi, @modamaan,
Conflicts have been detected against the base branch. Please rebase your branch against the base branch.


This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there.

@modamaan
Copy link
Contributor Author

modamaan commented Nov 6, 2024

@nihal467 now only focus on desktop view, phone size i have facing issues

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (6)
src/components/ui/card.tsx (5)

5-19: Consider enhancing accessibility attributes.

While the implementation is solid, consider adding ARIA attributes to improve accessibility:

 <div
   ref={ref}
   className={cn(
     "rounded-xl border border-gray-200 bg-white text-gray-950 shadow dark:border-gray-800 dark:bg-gray-950 dark:text-gray-50",
     className,
   )}
+  role="article"
+  aria-label="Card container"
   {...props}
 />

32-43: Consider adding heading level flexibility.

The component currently uses a fixed h3 element. Consider adding a prop to allow different heading levels for better semantic structure:

 const CardTitle = React.forwardRef<
   HTMLHeadingElement,
-  React.HTMLAttributes<HTMLHeadingElement>
+  React.HTMLAttributes<HTMLHeadingElement> & {
+    as?: 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6'
+  }
->(({ className, ...props }, ref) => (
+>(({ className, as: Comp = 'h3', ...props }, ref) => (
-  <h3
+  <Comp
     ref={ref}
     className={cn("font-semibold leading-none tracking-tight", className)}
     {...props}
   />
 ));

56-63: Add documentation for padding behavior.

Consider adding a comment to explain the pt-0 class, as it's not immediately obvious why top padding is removed:

 const CardContent = React.forwardRef<
   HTMLDivElement,
   React.HTMLAttributes<HTMLDivElement>
 >(({ className, ...props }, ref) => (
+  // pt-0 is used to remove top padding when CardContent follows CardHeader
   <div ref={ref} className={cn("p-6 pt-0", className)} {...props} />
 ));

64-74: Consider adding alignment flexibility.

The footer currently uses items-center but might benefit from configurable alignment:

 const CardFooter = React.forwardRef<
   HTMLDivElement,
-  React.HTMLAttributes<HTMLDivElement>
+  React.HTMLAttributes<HTMLDivElement> & {
+    justify?: 'start' | 'center' | 'end' | 'between'
+  }
->(({ className, ...props }, ref) => (
+>(({ className, justify = 'start', ...props }, ref) => (
   <div
     ref={ref}
-    className={cn("flex items-center p-6 pt-0", className)}
+    className={cn(
+      "flex items-center p-6 pt-0",
+      {
+        'justify-start': justify === 'start',
+        'justify-center': justify === 'center',
+        'justify-end': justify === 'end',
+        'justify-between': justify === 'between',
+      },
+      className
+    )}
     {...props}
   />
 ));

76-83: Consider maintaining consistent export order.

The export order differs slightly from the declaration order. Consider maintaining consistency:

 export {
   Card,
   CardHeader,
-  CardFooter,
   CardTitle,
   CardDescription,
   CardContent,
+  CardFooter,
 };
src/components/Shifting/ShiftDetails.tsx (1)

305-421: Well-structured print preview layout with room for accessibility improvement

The print preview layout is well-organized with proper responsive design. Consider adding ARIA labels to improve accessibility for screen readers.

Add aria-label to the card for better accessibility:

 <Card
   id="section-to-print"
+  aria-label="Referral Letter Preview"
   className="print mx-auto w-full bg-white sm:mx-2 sm:my-2"
 >
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 39a5f15 and a052237.

📒 Files selected for processing (2)
  • src/components/Shifting/ShiftDetails.tsx (4 hunks)
  • src/components/ui/card.tsx (1 hunks)
🔇 Additional comments (5)
src/components/ui/card.tsx (3)

1-4: LGTM! Clean and minimal imports.

The imports are well-organized and include only the necessary dependencies.


20-31: LGTM! Well-structured header component.

The component follows consistent patterns and implements appropriate spacing using flex layout.


44-55: LGTM! Semantic and accessible description component.

The component uses appropriate semantic HTML and maintains consistent styling patterns.

src/components/Shifting/ShiftDetails.tsx (2)

8-9: LGTM: New component imports

The imports for PrintPreview and Card components are correctly structured and follow the project's import conventions.


449-449: LGTM: Clean and responsive button layout

The flex container implementation provides a clean and responsive layout for the action buttons, properly handling both mobile and desktop views.

src/components/Shifting/ShiftDetails.tsx Outdated Show resolved Hide resolved
src/components/Shifting/ShiftDetails.tsx Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (2)
src/components/Shifting/ShiftDetails.tsx (2)

321-324: Consider increasing QR code size for better scannability.

The current QR code size of 120 pixels might be too small for reliable scanning, especially when printed.

 <QRCodeSVG
   value={`${window.location.origin}/shifting/${data.id}`}
-  size={120}
+  size={160}
 />

338-408: Consolidate text size classes for better maintainability.

The repeated text size classes (text-xs sm:text-sm) across multiple elements could be consolidated using a parent class or a utility class.

+<div className="text-xs sm:text-sm">
   <p>
-    className="text-xs sm:text-sm"
     {t("name")}: {patientData?.name}
   </p>
   <p>
-    className="text-xs sm:text-sm"
     {t("age")}: {formatPatientAge(patientData, true)}
   </p>
   // ... other paragraphs
+</div>
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between a052237 and a4f329a.

📒 Files selected for processing (1)
  • src/components/Shifting/ShiftDetails.tsx (4 hunks)
🧰 Additional context used
📓 Learnings (1)
src/components/Shifting/ShiftDetails.tsx (1)
Learnt from: modamaan
PR: ohcnetwork/care_fe#8954
File: src/components/Shifting/ShiftDetails.tsx:432-433
Timestamp: 2024-11-06T18:12:29.794Z
Learning: In `src/components/Shifting/ShiftDetails.tsx`, the `print` button is inside the `PrintPreview` component and the `close` button is outside, so aligning them with flexbox within the same container isn't feasible.
🔇 Additional comments (3)
src/components/Shifting/ShiftDetails.tsx (3)

8-10: LGTM! Import changes align with the new implementation.

The new imports support the PrintPreview functionality and card-based layout structure.

Also applies to: 18-18


432-433: LGTM! Button positioning accommodates component structure.

The button layout implementation considers the component structure where the print button is inside PrintPreview and the close button is outside.


449-449: LGTM! Improved button layout and user feedback.

The flex layout provides better button alignment, and the tooltip implementation enhances user experience by explaining disabled states.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changes required merge conflict pull requests with merge conflict
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix: referral letter responsiveness
4 participants